[fix](job scheduler) Record task finish time on execution failure - #66232
Merged
Conversation
Record the finish time when a task enters FAILED through the execution exception path, so MTMV failures expose complete terminal metadata and a usable duration. Key changes: - Set finishTimeMs in AbstractTask.onFail(). - Cover runTask() exception handling in AbstractJobStatusTest. Unit Test: - AbstractJobStatusTest: 13 tests passed.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
morrySnow
changed the title
Contributor
|
Add the PR that introduces the issue to the "related pr" section |
Contributor
TPC-H: Total hot run time: 29483 ms |
Contributor
TPC-DS: Total hot run time: 177248 ms |
Contributor
ClickBench: Total hot run time: 25.2 s |
CalvinKirs
approved these changes
Jul 29, 2026
Contributor
Author
|
run feut |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
Author
|
run p0 |
Contributor
Author
|
run cloud_p0 |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Member
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review complete: no blocking findings. Approval opinion for 90381bc804de4084ff7fe9f7b3b73ea5019dae3a.
Critical checkpoint conclusions:
- Goal, correctness, and proof: the production change fills the existing
finishTimeMsfield in the no-argument failure path used byAbstractTask.runTask()after an execution exception. The new unit test executes that path and verifiesFAILED, a non-null finish time, and the preserved exception message. - Scope and clarity: the production change is a single focused assignment and aligns
onFail()with the existingonFail(String)andonSuccess()terminal paths. - Concurrency and lifecycle: task execution remains on the existing executor paths, with no new threads, locks, lock ordering, or resource ownership. For MTMV, the timestamp is assigned before the job callback and before
MTMVTask.after()adds the task to persisted history, so FinishTime and DurationMs observe the terminal value. - Parallel paths and compatibility: the message-bearing failure path and success path already record finish time, and the relevant subclass overrides delegate to the base implementation. No configuration, FE/BE protocol, function symbol, storage format, or rolling-upgrade contract changes.
- Error handling, persistence, and observability: the caught exception message remains intact; the existing serialized
ftmfield is populated before MTMV history/edit-log handling, with no new persistence format. This directly repairs the task TVF metadata described by the PR. - Performance: the only runtime cost is one wall-clock read on a failure-only path.
- Test coverage and results: the added JUnit coverage is targeted and deterministic. Current PR checks show FE UT, CheckStyle, COMPILE, and the applicable regression suites passing. I did not run local builds or tests because the authoritative review instructions prohibit builds in this runner.
- Other applicability: there are no transaction/data-visibility, configuration, security, static-initialization, or FE-to-BE variable propagation changes.
- User focus: no additional user-provided review focus was supplied; the complete two-file PR and its upstream/downstream task lifecycle were reviewed.
No inline comments are proposed.
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 30, 2026
…6232) ### What problem does this PR solve? When a task fails through the execution exception path, `AbstractTask.runTask()` calls the no-argument `onFail()`, which marked the task FAILED without recording `finishTimeMs`. MTMV failed refresh tasks therefore exposed an empty FinishTime and could not calculate DurationMs. relate PR: #27703 ### What changes were made? - Record `finishTimeMs` when `AbstractTask.onFail()` transitions a task to FAILED. - Add a regression unit test covering the `runTask()` exception path and preserving the error message. ### Testing - `AbstractJobStatusTest`: 13 tests passed.
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 30, 2026
…6232) ### What problem does this PR solve? When a task fails through the execution exception path, `AbstractTask.runTask()` calls the no-argument `onFail()`, which marked the task FAILED without recording `finishTimeMs`. MTMV failed refresh tasks therefore exposed an empty FinishTime and could not calculate DurationMs. relate PR: #27703 ### What changes were made? - Record `finishTimeMs` when `AbstractTask.onFail()` transitions a task to FAILED. - Add a regression unit test covering the `runTask()` exception path and preserving the error message. ### Testing - `AbstractJobStatusTest`: 13 tests passed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
When a task fails through the execution exception path,
AbstractTask.runTask()calls the no-argumentonFail(), which marked the task FAILED without recordingfinishTimeMs. MTMV failed refresh tasks therefore exposed an empty FinishTime and could not calculate DurationMs.relate PR: #27703
What changes were made?
finishTimeMswhenAbstractTask.onFail()transitions a task to FAILED.runTask()exception path and preserving the error message.Testing
AbstractJobStatusTest: 13 tests passed.